Skip to content

Conversation

@tnull
Copy link
Collaborator

@tnull tnull commented Aug 22, 2025

Based on lightningdevkit/vss-client#40

In this PR we'll fix minor issues with the data encryption and key obfuscation scheme currently employed by VssStore.

As these are breaking changes, we'll also include a migration procedure as part of this PR. Will be in draft until we have all parts ready.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Aug 22, 2025

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@tnull tnull marked this pull request as draft August 22, 2025 09:14
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from ac8ae88 to 3490f2a Compare August 22, 2025 09:53
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 6aa9b81 to bec8829 Compare November 6, 2025 11:11
We bump our `vss-client` dependency to include the changes to the
`StorableBuilder` interface.

Previously, we the `vss-client` didn't allow to set `ChaCha20Poly1305RFC`'s `aad` field,
which had the `tag` not commit to any particular key. This would allow a
malicious VSS provider to substitute blobs stored under a different key
without the client noticing.

Here, we now set the `aad` field to the key under which the `Storable`
will be stored, ensuring that the retrieved data was originally stored
under the key we expected.

We also account for `StorableBuilder` now taking `data_decryption_key`
by reference on `build`/`deconstruct`.
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from bec8829 to e977a6d Compare November 6, 2025 11:41
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from 1d69408 to e582bc1 Compare November 6, 2025 11:45
tnull added 2 commits November 6, 2025 12:51
We recently dropped the `Retry` logic in `vss-client-ng`, and rather now
lean on `reqwest`'s retry logic. Here, we account for the corresponding
interface changes.
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from e582bc1 to c7fc423 Compare November 6, 2025 11:56
tnull added 3 commits November 6, 2025 12:57
We previously attempted to drop the internal runtime from `VssStore`,
resulting into blocking behavior. While we recently made changes that
improved our situation (having VSS CI pass again pretty reliably), we
just ran into yet another case where the VSS CI hung (cf.
https://github.com/lightningdevkit/vss-server/actions/runs/19023212819/job/54322173817?pr=59).

Here we attempt to restore even more of the original pre-
ab3d78d / lightningdevkit#623 behavior to get rid of
the reappearing blocking behavior, i.e., only use the internal runtime
in `VssStore`.
Now that we rely on `reqwest` v0.12.* retry logic as well as client-side
timeouts, we can address the remaining TODOs here and simply drop the
redundant `tokio::timeout`s we previously added as a safeguard to
blocking tasks (even though in the worst cases we saw they never
actually fired).
@tnull tnull force-pushed the 2025-08-upgrade-vss-encryption-obfuscation-scheme branch from c7fc423 to dd9a9e0 Compare November 6, 2025 11:57
As LDK currently will still panic and crash if we'd ever fail a `write`
operation, we here considerably bump the defaults set in `VssClient` to
at least make this less likely. Longer term, we still hope to mitigate
the issue by moving to the async-persist flow.
@tnull tnull moved this to Goal: Merge in Weekly Goals Nov 6, 2025
@tnull tnull self-assigned this Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Goal: Merge

Development

Successfully merging this pull request may close these issues.

2 participants